-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Detect .NET SDK & VSLANG Custom Language Settings & Apply To MSBuild #8503
Conversation
…other programs on the same console. Note that we could probably do this better. I tried to create a function to do so but because the SetConsoleUI is static this was the least ugly/simple way to do it. Open to other proposals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two things:
- Can you expand on the breaking change some more, please? Is it only that we now respect VSLANG and will emit UTF-8 when it is specified?
- Can you test this in the SDK when opted into MSBuild Server? I'm especially curious about the "do a build/start the server with no
DOTNET_CLI_UI_LANGUAGE
, then set it, then do another build" case.
Co-authored-by: Rainer Sigwald <raines@microsoft.com>
When you use If we didn't do that (which is what the SDK is currently doing): probably it will work for some versions, but maybe with some encoding bugs from Windows. Maybe it will fail/reject the encoding in very early versions. Outside of VS at the very least, if you set the environment variable without the encoding it would give you a bunch of ??? or garbled missing-no's. For VS it is more ... hrmmm. For non-Windows it should not matter, encoding is not an issue. For Win 8 and below setting the environment variables will probably result in garbage outputs until the user manually edits the code page. Thinking about it more, maybe we should try to set the language specific encoding for these languages. But each language has its own quirks with characters output by MSBuild that probably won't work well, and emojis (like the live logger) will be completely broken. In addition, I need to get to your point in |
This is waiting for dotnet/sdk#30963 to be merged. Once that's merged, I will ping again for review. |
…f8 dont get it by default unless opted in and introduce encoding restorer to prevent msbuild from changing the encoding.
…er takes care of this
Co-Authored-By: Forgind <12969783+Forgind@users.noreply.github.com>
…s doced as being in net 472
For 1) Now, we only set utf-8 if its officially supported or opted in. And we also restore the encoding. So the breaking change is that if VS doesn't set the encoding for win 8 or below then the characters could become garbled for non-en scenarios. Let's discuss this. For 2) It doesn't work and that may relate to the VS issue in 1. This is because xmake is skipped in these cases per @Forgind. I've pinged you offline @rainersigwald. |
@rokonec what do you think the best way to change the stdout codepage in the msbuild client is, based primarily on an environment variable? Should it be done in the client init code, or maybe we should add a new |
@rainersigwald This is good to consider, though we also must consider the VS Case not calling into XMake. What about BuildManager's BeginBuild methods? cc @Forgind |
If you're using the API, the log events should always be standard 16-bit UCS-2 .NET strings and shouldn't care about codepage one way or another. Does the "output window" in VS have problems with encodings in these cases? |
Nope! Tested it myself at least and it didn't.... there were issues but they all came from calling into So @rokonec sorry for the delay, please let us know your opinion on the above as it still doesn't work in msbuild server.
|
…lready and also fix a test to use en because bild machines are apparently in french sometimes???
I would recommend to move whole logic around Line 236 in 5a06292 so it will become on both, classic and server path. That shall cause to properly set However, if it does not work "fine" for server scenarios, please focus on another aspects of this PR and I volunteer to fixing it for server in another consequent PR. |
The encoding needs to be added to logging as well before this is merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to go
… before a debugger is attached so that build errors wont have a bad encoding
When working on this I happened to discover a deadlock (I think) that is unrelated to my changes. You have to set This happens even on main without my changes, then MSBuild will hang forever in a deadlock waiting for the _syncLock. If you press ctrl-c it will say |
🥳 |
Co-authored-by: Forgind <12969783+Forgind@users.noreply.github.com>
…sbuild into nagilson-cli-language
…lture as the existing code could change the culture to the fallback culture instead of the original culture
Fixes #1596
Changes Made
SetConsoleUI now calls into a helper which sets the encoding to support non-en languages and checks if an environment variable exists to change the language to.
Testing
Setting DOTNET_CLI_UI_LANGUAGE=ja now changes msbuild correctly:
![image](https://user-images.githubusercontent.com/23152278/221717457-7dd1e877-d317-4eb3-82f7-69f639332f40.png)
Doing a complicated build (aka building MSBuild) to use multiple threads shows other threads seem to use the same UI culture:
See that chcp remains the same after execution:
![image](https://user-images.githubusercontent.com/23152278/221720974-b5a0b6f5-aaa2-40b2-9c9f-fc5baaa6d9a8.png)
(Was set to 65001 temporarily but back to the original page before execution.)
Notes
Much of this code is a port of this code: dotnet/sdk#29755
There are some details about the code here.
[!] In addition, it will introduce a breaking change for msbuild just like the SDK.
The break is documented here for the sdk: dotnet/docs#34250